Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update to net46 #526

Merged
merged 1 commit into from
Jan 1, 2023
Merged

Update to net46 #526

merged 1 commit into from
Jan 1, 2023

Conversation

danmoseley
Copy link
Contributor

@danmoseley danmoseley commented Nov 27, 2022 •
edited
Loading

.NET 4.5 has been out of support for 6 years and the targeting pack is no longer available. The repo can only be built by those who happen to already have it. I don't..

.NET 4.6 is the latest version potentially in support. https://learn.microsoft.com/en-US/lifecycle/faq/dotnet-framework

  1. Change to target .NET 4.6
  2. Remove custom Array.Empty as it's in .NET 4.6.
  3. Future proof defines with _OR_GREATER.
  4. Fix the dotnet.exe finder tests. DOTNET_ROOT is not guaranteed to be defined. The defaults are documented, use those otherwise.

All tests pass locally.

@danmoseley
Copy link
Contributor Author

I see this project is in maintenance mode. Neverthless I suggest to take this change, because currently the repo cannot be built by some people.

@codecov
Copy link

codecov bot commented Dec 2, 2022 •
edited
Loading

Codecov Report

Merging #526 (ea50465) into main (2193138) will increase coverage by 0.01%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main     #526      +/-   ##
==========================================
+ Coverage   82.30%   82.31%   +0.01%     
==========================================
  Files         106      105       -1     
  Lines        3306     3303       -3     
==========================================
- Hits         2721     2719       -2     
+ Misses        585      584       -1
Impacted Files Coverage Δ
src/CommandLineUtils/IO/Pager.cs 0.00% <0.00%> (ø)
.../CommandLineUtils/Properties/NullabilityHelpers.cs 0.00% <ø> (ø)
...rc/CommandLineUtils/Attributes/CommandAttribute.cs 86.66% <50.00%> (ø)
...CommandLineUtils/CommandLineApplication.Execute.cs 70.58% <66.66%> (ø)
src/CommandLineUtils/Utilities/DotNetExe.cs 81.25% <66.66%> (-6.25%) ⬇️
...mmandLineUtils/Abstractions/ValueParserProvider.cs 96.59% <100.00%> (ø)
...ils/Attributes/VersionOptionFromMemberAttribute.cs 95.45% <100.00%> (ø)
src/CommandLineUtils/CommandLineApplication.cs 93.58% <100.00%> (ø)
...Utils/Conventions/OptionAttributeConventionBase.cs 96.52% <100.00%> (ø)
...mandLineUtils/HelpText/DefaultHelpTextGenerator.cs 99.34% <100.00%> (ø)
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@natemcmaster
Copy link
Owner

Thanks for making a PR to keep things up to date. I don’t see any problem with moving forward to a modern version of .NET Framework.

Out of curiosity, can you elaborate on the problem? I was under the impression that the targeting packs could be installed via NuGet, which is enabled in this project here:

CommandLineUtils/Directory.Build.props

Line 58 in dd5de06

<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.2" PrivateAssets="All" />

@danmoseley
Copy link
Contributor Author

IIRC, it simply would not build within VS, I was directed to get the targeting pack, then discovered it has been removed from downloads. I didn't look further.

@danmoseley
Copy link
Contributor Author

Do you want to merge (if only to get it out of my active PR list 🙂)

@natemcmaster natemcmaster merged commit 3cb6db2 into natemcmaster:main Jan 1, 2023
@natemcmaster
Copy link
Owner

Thanks @danmoseley !

@danmoseley danmoseley deleted the update.to.net46 branch January 9, 2023 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natemcmaster natemcmaster Awaiting requested review from natemcmaster natemcmaster is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danmoseley @natemcmaster